sroa: Lift restriction that all_same optimization must give SSAValue#52338
sroa: Lift restriction that all_same optimization must give SSAValue#52338
Conversation
|
@nanosoldier |
aviatesk
left a comment
aviatesk
left a comment
There was a problem hiding this comment.
The base test suite seems to be successful on this PR.
|
The package evaluation job you requested has completed - possible new issues were detected. |
This restriction has been in there since this code was added in #44557. Unfortunately, I can't tell from the commit history why this restriction was added. It's possible that it was trying to avoid putting things into statement position that were not allowed in the phi block, but we have cleaned that up since (#50308 and related), so let's see if this restriction is still required, since I was seeing some suboptimial optimization results because of this.
aviatesk
force-pushed
the
kf/sroarelaxallsame
branch
from
5179b1c to
74b8b6a
Compare
|
@nanosoldier |
|
FWIW, I found the bug that showed up on PkgEval, but it unraveled a whole thread, so standby for that |
|
The package evaluation job you requested has completed - possible new issues were detected. |
|
Sanity check to make sure I actually fixed the bug: |
|
The package evaluation job you requested has completed - no new issues were detected. |
|
@nanosoldier |
|
The package evaluation job you requested has completed - possible new issues were detected. |
Keno
force-pushed
the
kf/sroarelaxallsame
branch
from
ae2d3cd to
05fbff4
Compare
Although the path that created the NewSSAValue in the PkgEval case shouldn't really be there, so also add a small optimization that renames ssa values more aggressively, so that we can actually test this.
Keno
force-pushed
the
kf/sroarelaxallsame
branch
from
05fbff4 to
50f64d0
Compare
|
@nanosoldier |
|
The package evaluation job you requested has completed - no new issues were detected. |
3 participants
This restriction has been in there since this code was added in #44557. Unfortunately, I can't tell from the commit history why this restriction was added. It's possible that it was trying to avoid putting things into statement position that were not allowed in the phi block, but we have cleaned that up since (#50308 and related), so let's see if this restriction is still required, since I was seeing some suboptimial optimization results because of this.